ncps - expand inline guest #2356
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2356 +/- ##
==========================================
+ Coverage 58.00% 58.03% +0.03%
==========================================
Files 153 153
Lines 12232 12241 +9
Branches 742 745 +3
==========================================
+ Hits 7095 7104 +9
Misses 5041 5041
Partials 96 96
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -208,6 +209,14 @@ export const buildHostedButtonOnApprove = ({ | |||
| merchant_id: merchantId, | |||
| context_id: data.orderID, | |||
| }), | |||
| }).then((response) => { | |||
| // The "Debit or Credit Card" button does not open a popup | |||
There was a problem hiding this comment.
It does sometimes open a popup though right? or are we saying in the ncps case it ALWAYS does the expand?
There was a problem hiding this comment.
it does sometimes open a popup - e.g. clicking on the debit or credit card button before second render. I tested this scenario and made sure a second popup isn't opened though it's not obvious to me just yet why that isn't a bug 🤔
There was a problem hiding this comment.
does it redirect inside the popup instead of the parent page?
There was a problem hiding this comment.
no it closes the popup and then full page redirects from the merchant site to the thank you page
There was a problem hiding this comment.
oh wow, so this handles both cases. That's great!
There was a problem hiding this comment.
yes! though I think what's happening under the hood is that this window.location = is forcing the popup to close in the beforeUnload event. Similar to:
- Click on paypal button (popup opens)
- open dev console and
window.location = 'https://example.com/' - Observe popup close
I think this is an OK tradeoff but wanted to be transparent about why this "works". An alternative would be only invoking window.location= if the popup window wasn't opened because of a prerender click (not sure what's involved in that but I bet it will be challenging / complex)
There was a problem hiding this comment.
would there be enough time to close it through onApprove?
There was a problem hiding this comment.
Definitely! But I think the correct way would be to not close the popup - e.g. redirect from within the popup to the thank you page (identical to the PayPal button functionality)
There was a problem hiding this comment.
e.g. I don't think there's a way to know that onApprove was invoked from a prerender-click context: I'll timebox it early next week and see if it's a quick win but otherwise I think we should go with what's in this PR - wdyt? I'm def open to other ideas
There was a problem hiding this comment.
i'm down for the bare minimum that matches what product is asking for. Whatever that is lol but feel free to do as much exploration as you want
Description
This PR is a partial revert of #2320, specifically: inline guest no longer opens a popup by default for ncps flows, but expands inline and redirects to a thank you page after
onApproveis invoked.Why are we making these changes?
There is an open incident regarding the discrepancy between inline guest expanding vs popup. Let me know if you'd like more info and I can point you in the right direction.
Reproduction Steps (if applicable)
Complete a checkout with a hosted button using inline guest. You should see inline guest expand inline, and redirect to a thank you page after the order is approved and captured.
Dependent Changes (if applicable)
❤️ Thank you!